Skip to content

Etapa 3: infraestructura para publicar mapas en vivo sin reiniciar - #93

Open
xusuxiang8 wants to merge 3 commits into
Bitcoindefi:mainfrom
xusuxiang8:feat/map-live-publish
Open

Etapa 3: infraestructura para publicar mapas en vivo sin reiniciar#93
xusuxiang8 wants to merge 3 commits into
Bitcoindefi:mainfrom
xusuxiang8:feat/map-live-publish

Conversation

@xusuxiang8

Copy link
Copy Markdown

Implementa infraestructura para publicar mapas en vivo sin reiniciar el servidor.

Cambios

  1. server/src/loadMaps.ts — Nuevo método reloadMapByNumber(mapNum):

    • Recarga un mapa individual desde disco
    • Reconstruye el runtime (tiles, exits, objetos, NPCs, triggers, metadata)
    • Reubica jugadores que quedan sobre tiles bloqueados (mapa 1, 50, 50)
    • Notifica a los clientes conectados en ese mapa vía socket
  2. api/src/server.ts — Nuevo endpoint:

    • POST /internal/game-data/maps/publish/:mapId — Activa la publicación de un mapa
    • Restringido a admins de game data
  3. Seguridad de jugadores:

    • Si un jugador está parado sobre un tile que pasó a ser bloqueado,
      se lo reubica automáticamente en la posición segura de fallback

Pendiente (depende de issues de mutación previas)

  • Integración con gameDataSync.ts para polling periódico
  • Invalidación de caché del cliente (requiere cambios en el frontend)

Closes #11

Comment thread server/src/loadMaps.ts
Comment on lines 288 to +291
}
}

/** Recarga un mapa individual desde disco y actualiza el runtime. */

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 Bug: reloadMapByNumber defined outside the class body — won't compile

The LoadMaps class already closes with } at line 289. The new reloadMapByNumber method is added at lines 291-398 outside the class, using object-literal method syntax terminated by a trailing comma (}, at line 398). This is a TypeScript syntax error: a top-level async reloadMapByNumber(...) {...}, is not valid, so the whole module fails to compile, and this.mapFilesExist/this.getMapDirectory would not resolve even if it did. Move the method inside the class: delete the } at line 289, place the method (with no trailing comma) before the class-closing brace, and keep module.exports = LoadMaps; after the class.

Move the method inside the class and remove the stray trailing comma.:

        });
    }

    /** Recarga un mapa individual desde disco y actualiza el runtime. */
    async reloadMapByNumber(mapNum: number): Promise<{ ok: boolean; playersAffected: number }> {
        // ... body unchanged ...
        return { ok: true, playersAffected: movedPlayers };
    }
}

module.exports = LoadMaps;
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

Comment thread server/src/loadMaps.ts
Comment on lines +377 to +387
for (const player of playersOnMap) {
const tile = vars.mapa[mapNum]?.[player.pos.y]?.[player.pos.x];
if (!tile || tile.blocked) {
const FallbackMap = 1, FallbackX = 50, FallbackY = 50;
player.map = FallbackMap;
player.pos = { x: FallbackX, y: FallbackY };
player.posX = FallbackX;
player.posY = FallbackY;
movedPlayers++;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Edge Case: Fallback tile (1,50,50) is not validated as walkable

Players standing on a newly-blocked tile are teleported to the hardcoded fallback (map 1, 50, 50), but there is no check that this destination tile exists and is not itself blocked. If map 1's tile (50,50) is blocked or out of bounds after a reload, players are relocated into an invalid/blocked position, potentially trapping them. Validate the fallback tile (vars.mapa[1]?.[50]?.[50] exists and !blocked) or choose a known-safe spawn, and handle the case where it is unavailable.

Guard the fallback destination.:

const FallbackMap = 1, FallbackX = 50, FallbackY = 50;
const fbTile = vars.mapa[FallbackMap]?.[FallbackY]?.[FallbackX];
if (fbTile && !fbTile.blocked) {
    player.map = FallbackMap;
    player.pos = { x: FallbackX, y: FallbackY };
    player.posX = FallbackX;
    player.posY = FallbackY;
    movedPlayers++;
}
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

Comment thread server/src/loadMaps.ts
Comment on lines +377 to +391
for (const player of playersOnMap) {
const tile = vars.mapa[mapNum]?.[player.pos.y]?.[player.pos.x];
if (!tile || tile.blocked) {
const FallbackMap = 1, FallbackX = 50, FallbackY = 50;
player.map = FallbackMap;
player.pos = { x: FallbackX, y: FallbackY };
player.posX = FallbackX;
player.posY = FallbackY;
movedPlayers++;
}
}

// Broadcast map reload to clients on this map
for (const player of playersOnMap) {
if (player.connection && player.connection.emit) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Bug: Relocated players still notified about the old map reload

playersOnMap is captured before repositioning, then reused for the broadcast loop. Players who were moved to the fallback map still receive emit("mapReloaded", { mapNum }) for a map they are no longer on, while their actual new position/map is never communicated to the client. Broadcast the reload only to players who remain on mapNum, and send an explicit teleport/position update to relocated players.

Only emit reload to players still on the map.:

for (const player of playersOnMap) {
    if (player.map === mapNum && player.connection && player.connection.emit) {
        player.connection.emit("mapReloaded", { mapNum: mapNum });
    }
}
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

Comment thread server/src/loadMaps.ts
vars.mapData[mapNum].pk = toNumber(metadata.pk);

// Handle player safety: move players off blocked tiles
const socket = require("./socket");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Unused socket require in reloadMapByNumber

const socket = require("./socket"); is declared but never used (the broadcast uses player.connection.emit directly). Remove the dead require to avoid confusion and an unnecessary module load.

Delete the unused line.:

// remove: const socket = require("./socket");
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown
Code Review 🚫 Blocked 0 resolved / 4 findings

Adds live map reloading infrastructure and an internal publish endpoint, but fails to compile because reloadMapByNumber is defined outside the LoadMaps class body and the fallback tile lacks walkability validation.

🚨 Bug: reloadMapByNumber defined outside the class body — won't compile

📄 server/src/loadMaps.ts:288-291 📄 server/src/loadMaps.ts:398-400

The LoadMaps class already closes with } at line 289. The new reloadMapByNumber method is added at lines 291-398 outside the class, using object-literal method syntax terminated by a trailing comma (}, at line 398). This is a TypeScript syntax error: a top-level async reloadMapByNumber(...) {...}, is not valid, so the whole module fails to compile, and this.mapFilesExist/this.getMapDirectory would not resolve even if it did. Move the method inside the class: delete the } at line 289, place the method (with no trailing comma) before the class-closing brace, and keep module.exports = LoadMaps; after the class.

Move the method inside the class and remove the stray trailing comma.
        });
    }

    /** Recarga un mapa individual desde disco y actualiza el runtime. */
    async reloadMapByNumber(mapNum: number): Promise<{ ok: boolean; playersAffected: number }> {
        // ... body unchanged ...
        return { ok: true, playersAffected: movedPlayers };
    }
}

module.exports = LoadMaps;
⚠️ Edge Case: Fallback tile (1,50,50) is not validated as walkable

📄 server/src/loadMaps.ts:377-387

Players standing on a newly-blocked tile are teleported to the hardcoded fallback (map 1, 50, 50), but there is no check that this destination tile exists and is not itself blocked. If map 1's tile (50,50) is blocked or out of bounds after a reload, players are relocated into an invalid/blocked position, potentially trapping them. Validate the fallback tile (vars.mapa[1]?.[50]?.[50] exists and !blocked) or choose a known-safe spawn, and handle the case where it is unavailable.

Guard the fallback destination.
const FallbackMap = 1, FallbackX = 50, FallbackY = 50;
const fbTile = vars.mapa[FallbackMap]?.[FallbackY]?.[FallbackX];
if (fbTile && !fbTile.blocked) {
    player.map = FallbackMap;
    player.pos = { x: FallbackX, y: FallbackY };
    player.posX = FallbackX;
    player.posY = FallbackY;
    movedPlayers++;
}
💡 Bug: Relocated players still notified about the old map reload

📄 server/src/loadMaps.ts:377-391

playersOnMap is captured before repositioning, then reused for the broadcast loop. Players who were moved to the fallback map still receive emit("mapReloaded", { mapNum }) for a map they are no longer on, while their actual new position/map is never communicated to the client. Broadcast the reload only to players who remain on mapNum, and send an explicit teleport/position update to relocated players.

Only emit reload to players still on the map.
for (const player of playersOnMap) {
    if (player.map === mapNum && player.connection && player.connection.emit) {
        player.connection.emit("mapReloaded", { mapNum: mapNum });
    }
}
💡 Quality: Unused socket require in reloadMapByNumber

📄 server/src/loadMaps.ts:371

const socket = require("./socket"); is declared but never used (the broadcast uses player.connection.emit directly). Remove the dead require to avoid confusion and an unnecessary module load.

Delete the unused line.
// remove: const socket = require("./socket");
🤖 Prompt for agents
Code Review: Adds live map reloading infrastructure and an internal publish endpoint, but fails to compile because reloadMapByNumber is defined outside the LoadMaps class body and the fallback tile lacks walkability validation.

1. 🚨 Bug: reloadMapByNumber defined outside the class body — won't compile
   Files: server/src/loadMaps.ts:288-291, server/src/loadMaps.ts:398-400

   The `LoadMaps` class already closes with `}` at line 289. The new `reloadMapByNumber` method is added at lines 291-398 *outside* the class, using object-literal method syntax terminated by a trailing comma (`},` at line 398). This is a TypeScript syntax error: a top-level `async reloadMapByNumber(...) {...},` is not valid, so the whole module fails to compile, and `this.mapFilesExist`/`this.getMapDirectory` would not resolve even if it did. Move the method inside the class: delete the `}` at line 289, place the method (with no trailing comma) before the class-closing brace, and keep `module.exports = LoadMaps;` after the class.

   Fix (Move the method inside the class and remove the stray trailing comma.):
           });
       }
   
       /** Recarga un mapa individual desde disco y actualiza el runtime. */
       async reloadMapByNumber(mapNum: number): Promise<{ ok: boolean; playersAffected: number }> {
           // ... body unchanged ...
           return { ok: true, playersAffected: movedPlayers };
       }
   }
   
   module.exports = LoadMaps;

2. ⚠️ Edge Case: Fallback tile (1,50,50) is not validated as walkable
   Files: server/src/loadMaps.ts:377-387

   Players standing on a newly-blocked tile are teleported to the hardcoded fallback (map 1, 50, 50), but there is no check that this destination tile exists and is not itself blocked. If map 1's tile (50,50) is blocked or out of bounds after a reload, players are relocated into an invalid/blocked position, potentially trapping them. Validate the fallback tile (`vars.mapa[1]?.[50]?.[50]` exists and `!blocked`) or choose a known-safe spawn, and handle the case where it is unavailable.

   Fix (Guard the fallback destination.):
   const FallbackMap = 1, FallbackX = 50, FallbackY = 50;
   const fbTile = vars.mapa[FallbackMap]?.[FallbackY]?.[FallbackX];
   if (fbTile && !fbTile.blocked) {
       player.map = FallbackMap;
       player.pos = { x: FallbackX, y: FallbackY };
       player.posX = FallbackX;
       player.posY = FallbackY;
       movedPlayers++;
   }

3. 💡 Bug: Relocated players still notified about the old map reload
   Files: server/src/loadMaps.ts:377-391

   `playersOnMap` is captured before repositioning, then reused for the broadcast loop. Players who were moved to the fallback map still receive `emit("mapReloaded", { mapNum })` for a map they are no longer on, while their actual new position/map is never communicated to the client. Broadcast the reload only to players who remain on `mapNum`, and send an explicit teleport/position update to relocated players.

   Fix (Only emit reload to players still on the map.):
   for (const player of playersOnMap) {
       if (player.map === mapNum && player.connection && player.connection.emit) {
           player.connection.emit("mapReloaded", { mapNum: mapNum });
       }
   }

4. 💡 Quality: Unused `socket` require in reloadMapByNumber
   Files: server/src/loadMaps.ts:371

   `const socket = require("./socket");` is declared but never used (the broadcast uses `player.connection.emit` directly). Remove the dead require to avoid confusion and an unnecessary module load.

   Fix (Delete the unused line.):
   // remove: const socket = require("./socket");

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Etapa 3: publicar mapas en vivo sin reiniciar el server

1 participant